Initial support for pixel registry #6878
Conversation
ladamski
force-pushed
the
la/pixel-schema
branch
from
e1030b3 to
700ea42
Compare
This reverts commit 7e35fd0.
| # Lint step for push (fail on error), for PRs run fix instead | ||
| - name: Check or Fix lint | ||
| id: lint | ||
| run: | | ||
| if [[ "${{ github.event_name }}" == "pull_request" ]]; then | ||
| npm run lint.fix || true | ||
| else | ||
| npm run lint | ||
| fi | ||
| working-directory: PixelDefinitions | ||
|
|
||
| # Commit and push changes if lint.fix was run and made changes | ||
| - name: Commit and push linter fixes (PRs only) | ||
| if: github.event_name == 'pull_request' | ||
| run: | | ||
| git config --global user.name "github-actions[bot]" | ||
| git config --global user.email "github-actions[bot]@users.noreply.github.com" | ||
| git add PixelDefinitions/ | ||
| if ! git diff --cached --quiet; then | ||
| git commit -m "chore: auto-fix lint errors" | ||
| git push | ||
| fi |
There was a problem hiding this comment.
Let's check with the Android team if they are ok with automated commits. Windows opted into this, but Apple preferred a simple diff output on error: https://app.asana.com/1/137249556945/project/414709148257752/task/1210957466937460?focus=true
There was a problem hiding this comment.
Bringing my comment:
- lint should always run in PRs and develop and should be added to our pre-commit hook (this las bit needs added).
- lint-fix should be run manually by the owner.
- No automated commits anywhere.
| { | ||
| "epbf_android": { | ||
| "description": "User sent broken site report", | ||
| "owners": [], |
There was a problem hiding this comment.
I think this will fail validation with the latest schema release (owners should not be empty). Maybe Kate can be the owner for Android breakage pixels? Let's ask her in Asana.
| @@ -0,0 +1,28 @@ | |||
| { | |||
There was a problem hiding this comment.
We already have a package.json, do we really need a second one?
|
|
||
| on: | ||
| push: | ||
| branches: [develop] |
There was a problem hiding this comment.
this is not needed, validation should only run on PR commits and not when the PR is merged. The PR won’t be merged if there are lint issues.
| if [[ "${{ github.event_name }}" == "pull_request" ]]; then | ||
| npm run lint.fix || true | ||
| else | ||
| npm run lint |
There was a problem hiding this comment.
We can do this as part of our https://github.com/duckduckgo/Android/blob/develop/.github/workflows/ci.yml that runs on each PR merge. That would also prevent this workflow from running in parallel.
Task/Issue URL: https://app.asana.com/1/137249556945/project/1171671347221384/task/1210907475542624
Description
Initial support for https://app.asana.com/1/137249556945/project/1209805270658160/list/1209805324244987
Specific subtask for this PR: https://app.asana.com/1/137249556945/task/1210907475542626
Steps to test this PR
https://app.asana.com/1/137249556945/task/1209901985192016?focus=true using ".../Android/PixelDefinitions/" as the target directory where specified
Feature 1
UI changes